Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PB-1216: Add warning on sharing when there is a local files. #1136

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

ismailsunni
Copy link
Contributor

@ismailsunni ismailsunni commented Nov 26, 2024

image

Let me know if there is suggestion for the warning text.

Test link

Copy link

cypress bot commented Nov 26, 2024

web-mapviewer    Run #3899

Run Properties:  status check passed Passed #3899  •  git commit 901719feaa: PB-1216: sort map actions for easier reading.
Project web-mapviewer
Branch Review pb-1216-warning-sharing-local-file
Run status status check passed Passed #3899
Run duration 05m 10s
Commit git commit 901719feaa: PB-1216: sort map actions for easier reading.
Committer Ismail Sunni
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 21
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 212
View all changes introduced in this branch ↗︎

Copy link
Contributor

@pakb pakb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make more sense to have the "Share link input" and its button with this yellow-ish color, and a tooltip that shows when we hover it stating what you've added in the warning message.

Having a warning message popping up each time you open the share section is not ideal and not comfortable for users IMO

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants